Add VM snapshot functionality for time-travel debugging #29
Conversation
|
Resolves Issue #26 |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughA new snapshot serialization module captures and restores complete VM runtime state including project data, targets, monitors, threads, timers, and IO devices. The VirtualMachine class gains Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant VM as VirtualMachine
participant Snapshot as snapshot.js
participant Targets
participant Monitors
participant Threads
participant IODevices
participant SB3 as sb3 module
rect rgb(240, 248, 255)
Note over User,SB3: Serialization Flow
User->>VM: takeSnapshot()
VM->>Snapshot: serialize(runtime)
Snapshot->>SB3: sb3.serialize()
SB3-->>Snapshot: project data
Snapshot->>Targets: collect state
Targets-->>Snapshot: positions, variables
Snapshot->>Monitors: collect state
Monitors-->>Snapshot: monitor values
Snapshot->>Threads: collect state
Threads-->>Snapshot: stacks, frames, warp state
Snapshot->>IODevices: collect state
IODevices-->>Snapshot: timers, keyboard, mouse, video
Snapshot-->>VM: { project, runtimeState }
VM-->>User: snapshot
end
rect rgb(240, 255, 240)
Note over User,SB3: Deserialization Flow
User->>VM: loadSnapshot(snapshotData)
VM->>Snapshot: deserialize(runtime, snapshot)
Snapshot->>Snapshot: validate format
Snapshot->>SB3: sb3.deserialize()
SB3-->>Snapshot: runtime loaded
Snapshot->>Targets: restoreTargetRuntimeState()
Snapshot->>Monitors: restoreMonitorState()
Snapshot->>Threads: restoreThreadState()
Snapshot->>IODevices: restoreIODeviceState()
Snapshot-->>VM: restore complete
VM->>VM: emit SNAPSHOT_LOADED
VM-->>User: success
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@8to16 omg look it doesn't hallucinate no more |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/serialization/snapshot.js (4)
156-197: Add error handling for individual restoration steps.The deserialization process calls multiple restoration helpers (lines 172, 177, 182, 187, 192) in sequence without individual error handling. If any helper throws an exception, the entire snapshot load fails, which may be too strict.
Consider whether partial restoration should be allowed if certain components fail (e.g., thread state restoration fails but target positions should still be restored). Here's a more resilient approach:
🔎 Proposed enhancement for error resilience
return sb3.deserialize(snapshot.project, runtime) .then(({targets, extensions}) => { // Restore target runtime state if (snapshot.runtimeState.targets) { - restoreTargetRuntimeState(runtime, snapshot.runtimeState.targets); + try { + restoreTargetRuntimeState(runtime, snapshot.runtimeState.targets); + } catch (e) { + log.error('Failed to restore target runtime state:', e); + } } // Restore monitor state if (snapshot.runtimeState.monitors) { - restoreMonitorState(runtime, snapshot.runtimeState.monitors); + try { + restoreMonitorState(runtime, snapshot.runtimeState.monitors); + } catch (e) { + log.error('Failed to restore monitor state:', e); + } } // Similar try-catch for other restoration steps... return {targets, extensions}; });Alternatively, if complete restoration is required, document that partial failures are not acceptable and ensure all helpers validate their inputs defensively.
204-247: Minor: Improve validation for position and costume restoration.Two small improvements for defensive coding:
Line 213: The
setXYcall checks iftargetState.xis defined but uses bothxandy. Ifxis defined butyis not (unusual but possible), this could cause unexpected behavior.Line 217:
setCostumeis called withtargetState.currentCostumewithout validating that the index is within bounds for the target's costume list.🔎 Proposed defensive improvements
- if (typeof targetState.x !== 'undefined') target.setXY(targetState.x, targetState.y); + if (typeof targetState.x !== 'undefined' && typeof targetState.y !== 'undefined') { + target.setXY(targetState.x, targetState.y); + } - if (typeof targetState.currentCostume !== 'undefined') target.setCostume(targetState.currentCostume); + if (typeof targetState.currentCostume !== 'undefined') { + const maxCostume = target.getCostumes().length - 1; + const costumeIndex = Math.max(0, Math.min(targetState.currentCostume, maxCostume)); + target.setCostume(costumeIndex); + }These changes are optional but would make restoration more robust against malformed or outdated snapshots.
332-344: Consider time drift implications for timer restoration.The timer restoration (lines 332-344) directly restores timing values including
currentMSecs,currentStepTime, andprojectTimer. If a snapshot is loaded minutes or hours after creation, these absolute time values may cause unexpected behavior.For time-travel debugging, consider whether you want:
- Absolute time restoration: Keep current implementation (timers restore to snapshot values).
- Relative time restoration: Adjust timers relative to current time (e.g., reset
projectTimerto 0).- Hybrid approach: Reset some timers (project timer) but preserve others (step timing for animation consistency).
Document the intended behavior for timer restoration, as it affects debugging workflows (e.g., "go back 30 seconds" vs. "restore exact timer state").
351-374: Document input device state restoration behavior.Restoring keyboard and mouse state (lines 352-366) may create mismatches between physical hardware state and the VM's restored state:
- Keyboard: If keys were pressed in the snapshot but released physically, the VM thinks they're still pressed.
- Mouse: Restored position may not match the actual cursor position.
Consider documenting this behavior or offering options:
- Full restoration (current): Restore all input state exactly (good for true time-travel debugging).
- Partial restoration: Skip keyboard/mouse state and only restore video state (better for "load checkpoint" use cases where user expects current input state).
- Reset on first input: Restore snapshot state but clear it on first real user input to resync.
The current approach is reasonable for time-travel debugging, but users should understand that phantom key presses or mouse positions from the snapshot may occur until they interact with the project again.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/serialization/snapshot.jssrc/virtual-machine.js
🧰 Additional context used
🧬 Code graph analysis (2)
src/virtual-machine.js (1)
src/serialization/snapshot.js (1)
log(10-10)
src/serialization/snapshot.js (1)
src/virtual-machine.js (7)
require(20-20)require(24-24)require(25-25)require(26-26)require(28-28)Variable(21-21)log(13-13)
🔇 Additional comments (2)
src/serialization/snapshot.js (2)
17-61: Variable type constants are properly defined and correctly used.The
Variable.SCALAR_TYPEandVariable.LIST_TYPEconstants are defined as static getters insrc/engine/variable.js(returning''and'list'respectively) and match the usage in the serialize function. The defensive handling for list values at lines 57–58 withArray.isArray()check is appropriate.
275-325: Snapshot infrastructure and APIs are already well-established.The concern about "tight coupling to Thread internals" is overstated. The APIs used here are:
runtime._pushThread: Documented JSDoc method (runtime.js line 2083) used in tests and extensionsThread._StackFrame.create()and.release(): Intentionally exported asThread._StackFramewith comment "for extensions" (thread.js line 530), used in the codebaseThe snapshot format already has:
- Version management (version '1.0.0', checked during restore)
- Comprehensive test infrastructure (test/snapshot/lib.js, test/integration/tw-snapshots.js)
- Proper object pooling usage (correctly calls release() before creating new frames)
No changes needed; snapshot restoration is properly tested and managed.
| // Capture monitor state | ||
| if (runtime._monitorState) { | ||
| runtimeState.monitors = runtime._monitorState.valueSeq().toArray().map(monitor => ({ | ||
| id: monitor.get('id'), | ||
| value: monitor.get('value'), | ||
| visible: monitor.get('visible'), | ||
| mode: monitor.get('mode'), | ||
| x: monitor.get('x'), | ||
| y: monitor.get('y') | ||
| })); | ||
| } | ||
|
|
||
| // Capture thread execution state | ||
| runtimeState.threads = runtime.threads.map(thread => ({ | ||
| topBlock: thread.topBlock, | ||
| stack: [...thread.stack], | ||
| stackFrames: thread.stackFrames.map(frame => ({ | ||
| warpMode: frame.warpMode, | ||
| isLoop: frame.isLoop, | ||
| reporting: frame.reporting, | ||
| reported: frame.reported, | ||
| waitingReporter: frame.waitingReporter, | ||
| params: frame.params ? Object.assign({}, frame.params) : null, | ||
| executionContext: frame.executionContext ? Object.assign({}, frame.executionContext) : null | ||
| })), | ||
| status: thread.status, | ||
| targetId: thread.target ? thread.target.id : null, | ||
| stackClick: thread.stackClick, | ||
| updateMonitor: thread.updateMonitor, | ||
| warpTimer: thread.warpTimer ? { | ||
| startTime: thread.warpTimer.startTime, | ||
| timeElapsed: thread.warpTimer.timeElapsed() | ||
| } : null | ||
| })); | ||
|
|
||
| // Capture runtime timing state | ||
| runtimeState.timers = { | ||
| currentMSecs: runtime.currentMSecs, | ||
| currentStepTime: runtime.currentStepTime, | ||
| _lastStepTime: runtime._lastStepTime | ||
| }; | ||
|
|
||
| // Capture clock IO device state | ||
| if (runtime.ioDevices && runtime.ioDevices.clock) { | ||
| runtimeState.timers.projectTimer = runtime.ioDevices.clock._projectTimer; | ||
| runtimeState.timers.paused = runtime.ioDevices.clock._paused; | ||
| runtimeState.timers.pausedTime = runtime.ioDevices.clock._pausedTime; | ||
| } | ||
|
|
||
| // Capture IO device state | ||
| runtimeState.ioDevices = {}; | ||
|
|
||
| // Capture keyboard state | ||
| if (runtime.ioDevices && runtime.ioDevices.keyboard) { | ||
| runtimeState.ioDevices.keyboard = { | ||
| keysPressed: [...runtime.ioDevices.keyboard._keysPressed], | ||
| lastKeyPressed: runtime.ioDevices.keyboard.lastKeyPressed | ||
| }; | ||
| } | ||
|
|
||
| // Capture mouse state | ||
| if (runtime.ioDevices && runtime.ioDevices.mouse) { | ||
| runtimeState.ioDevices.mouse = { | ||
| clientX: runtime.ioDevices.mouse._clientX, | ||
| clientY: runtime.ioDevices.mouse._clientY, | ||
| scratchX: runtime.ioDevices.mouse._scratchX, | ||
| scratchY: runtime.ioDevices.mouse._scratchY, | ||
| isDown: runtime.ioDevices.mouse._isDown, | ||
| buttons: Array.from(runtime.ioDevices.mouse._buttons || []) | ||
| }; | ||
| } | ||
|
|
||
| // Capture video state | ||
| if (runtime.ioDevices && runtime.ioDevices.video) { | ||
| runtimeState.ioDevices.video = { | ||
| skinId: runtime.ioDevices.video._skinId, | ||
| drawable: runtime.ioDevices.video._drawable, | ||
| ghost: runtime.ioDevices.video._ghost | ||
| }; | ||
| } |
There was a problem hiding this comment.
Significant coupling to private implementation details.
The serialize function extensively accesses underscore-prefixed private properties across multiple components:
- Line 64:
runtime._monitorState(private) - Lines 107-108:
clock._projectTimer,_paused,_pausedTime(private) - Lines 118-119:
keyboard._keysPressed,lastKeyPressed(private) - Lines 126-131:
mouse._clientX,_clientY,_scratchX,_scratchY,_isDown,_buttons(private) - Lines 138-140:
video._skinId,_drawable,_ghost(private)
This creates tight coupling to internal implementation details that may change without notice, breaking snapshot compatibility.
Consider:
- Public APIs: Where possible, use public getter methods instead of private properties.
- Documentation: Document which private APIs are dependencies for snapshot format.
- Versioning: The version field at line 23 is good, but consider a compatibility checking mechanism for handling format changes.
- Abstraction: Consider adding explicit serialization methods to IO devices (e.g.,
device.getState()/device.setState()).
The current approach is pragmatic for an initial implementation, but plan for a more robust solution if this feature is heavily used or needs long-term snapshot compatibility.
🤖 Prompt for AI Agents
In src/serialization/snapshot.js around lines 63 to 142 the serializer directly
reads underscore-prefixed private properties from runtime monitors, clock,
keyboard, mouse and video which couples snapshots to internal implementation;
replace these direct property accesses with calls to public APIs (use existing
getters or add and call new device.getState()/setState() or
monitor.serialize()/deserialize() methods), add safe fallbacks when a public
getter is not present (e.g., call the getter if available, otherwise skip or
log), and update the snapshot format documentation/versioning and add a
compatibility check so future internal changes don't break snapshot loading.
| const restoreMonitorState = function (runtime, monitorStates) { | ||
| monitorStates.forEach(monitorState => { | ||
| // Update monitor using runtime's monitor update system | ||
| const monitorRecord = new Map([ | ||
| ['id', monitorState.id], | ||
| ['value', monitorState.value], | ||
| ['visible', monitorState.visible], | ||
| ['mode', monitorState.mode], | ||
| ['x', monitorState.x], | ||
| ['y', monitorState.y] | ||
| ]); | ||
|
|
||
| runtime.requestUpdateMonitor(monitorRecord); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for requestUpdateMonitor definition and usage
rg -n 'requestUpdateMonitor' --type=js -B 3 -A 10Repository: OmniBlocks/scratch-vm
Length of output: 10276
🏁 Script executed:
# Also search for the serialization context - look for monitor-related code
rg -n 'monitorState|restoreMonitorState' --type=js -B 2 -A 5Repository: OmniBlocks/scratch-vm
Length of output: 49475
🏁 Script executed:
# Check if there's any Immutable.js usage pattern for Map objects
rg -n 'Immutable\.Map|new Map' src/serialization/snapshot.js -B 2 -A 2Repository: OmniBlocks/scratch-vm
Length of output: 312
🏁 Script executed:
# Check the imports at the top of snapshot.js
head -50 src/serialization/snapshot.jsRepository: OmniBlocks/scratch-vm
Length of output: 1657
🏁 Script executed:
# Also verify the exact API that requestUpdateMonitor expects by looking at its implementation more closely
sed -n '3186,3210p' src/engine/runtime.jsRepository: OmniBlocks/scratch-vm
Length of output: 1197
🏁 Script executed:
# Check what Map is being used in the runtime.js file
rg -n "^const.*Map|^const.*Immutable|import.*Map" src/engine/runtime.js | head -20Repository: OmniBlocks/scratch-vm
Length of output: 145
🏁 Script executed:
# Check if MonitorRecord is defined and what it is
rg -n "MonitorRecord" src/engine/runtime.js -B 2 -A 2 | head -30Repository: OmniBlocks/scratch-vm
Length of output: 343
🏁 Script executed:
# Check if snapshot.js imports Immutable or Map from anywhere
rg -n "require.*immutable|import.*immutable|Map" src/serialization/snapshot.js | head -20Repository: OmniBlocks/scratch-vm
Length of output: 143
🏁 Script executed:
# Look at how other files in the serialization module handle Maps
rg -n "new Map\|Map({" src/serialization/*.jsRepository: OmniBlocks/scratch-vm
Length of output: 167
🏁 Script executed:
# Search for Map usage in serialization files more carefully
rg -n "Map(" src/serialization/Repository: OmniBlocks/scratch-vm
Length of output: 123
🏁 Script executed:
# Also check sb2.js to see how it creates monitor records
rg -n "MonitorRecord" src/serialization/sb2.js -B 2 -A 5Repository: OmniBlocks/scratch-vm
Length of output: 849
🏁 Script executed:
# Check what MonitorRecord is and how it's defined
cat src/engine/monitor-record.jsRepository: OmniBlocks/scratch-vm
Length of output: 678
🏁 Script executed:
# Also check how execute.js and blocks.js create Maps for requestUpdateMonitor
rg -n "requestUpdateMonitor" src/engine/execute.js -B 2 -A 8 | head -30Repository: OmniBlocks/scratch-vm
Length of output: 523
🏁 Script executed:
# Check if snapshot.js imports anything from immutable
cat src/serialization/snapshot.js | grep -E "require|import" | head -20Repository: OmniBlocks/scratch-vm
Length of output: 278
🏁 Script executed:
# Check how runtime.js defines and imports Map
rg -n "Map" src/engine/runtime.js | head -20Repository: OmniBlocks/scratch-vm
Length of output: 1331
🏁 Script executed:
# Check if Immutable is imported anywhere in execute.js to see how Map is used there
head -30 src/engine/execute.js | grep -E "require|import|Map"Repository: OmniBlocks/scratch-vm
Length of output: 321
🏁 Script executed:
# Verify the exact usage in execute.js
sed -n '1,10p' src/engine/execute.jsRepository: OmniBlocks/scratch-vm
Length of output: 411
Import Map from immutable and use Map({...}) syntax.
At line 257, the code creates a native JavaScript Map, but requestUpdateMonitor expects an Immutable.js Map. The runtime will fail when calling .mergeWith() on line 3198 of runtime.js since native Map lacks this method.
Change snapshot.js to:
- Add import at the top:
const {Map} = require('immutable'); - Replace
new Map([...])withMap({...})syntax to match the pattern used in execute.js (line 103), blocks.js (line 714), and target.js (line 355).
Example fix
const {Map} = require('immutable');
// In restoreMonitorState function:
const monitorRecord = Map({
id: monitorState.id,
value: monitorState.value,
visible: monitorState.visible,
mode: monitorState.mode,
x: monitorState.x,
y: monitorState.y
});🤖 Prompt for AI Agents
In src/serialization/snapshot.js around lines 254 to 268, the code builds a
native JS Map but runtime.requestUpdateMonitor expects an Immutable.js Map;
import Immutable's Map at the top (const {Map} = require('immutable');) and
replace new Map([...]) with the Immutable Map({...}) object-literal factory form
for monitorRecord so requestUpdateMonitor receives an Immutable Map with the
expected methods.
| /** | ||
| * Take a snapshot of the current VM state for time-travel debugging. | ||
| * Captures complete runtime state including sprite positions, variables, | ||
| * running threads, timers, and IO device state. | ||
| * @return {object} Snapshot object containing all VM state. | ||
| */ | ||
| takeSnapshot () { | ||
| const snapshot = require('./serialization/snapshot'); | ||
| return snapshot.serialize(this.runtime); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the actual code at the specified lines
cat -n src/virtual-machine.js | sed -n '1920,1935p'Repository: OmniBlocks/scratch-vm
Length of output: 805
🏁 Script executed:
# Search for takeSnapshot usage in the codebase
rg -n 'takeSnapshot' --type=jsRepository: OmniBlocks/scratch-vm
Length of output: 114
🏁 Script executed:
# Look for performance considerations or async patterns with serialization
rg -n -B2 -A2 'serialize|snapshot' src/virtual-machine.js | head -50Repository: OmniBlocks/scratch-vm
Length of output: 2367
🏁 Script executed:
# Check if there are performance tests related to snapshots
fd -t f '.*test.*' -o '.*spec.*' | xargs rg -l 'snapshot|serialize' 2>/dev/null || echo "Checking test files..."Repository: OmniBlocks/scratch-vm
Length of output: 2221
🏁 Script executed:
# Search for calls to takeSnapshot (not just the definition)
rg -n 'takeSnapshot\s*\(' --type=js | grep -v '^src/virtual-machine.js:1927'Repository: OmniBlocks/scratch-vm
Length of output: 47
🏁 Script executed:
# Examine the snapshot serializer implementation
cat -n src/serialization/snapshot.js | head -80Repository: OmniBlocks/scratch-vm
Length of output: 3406
🏁 Script executed:
# Check the test file for snapshot behavior
cat test/integration/tw-snapshots.jsRepository: OmniBlocks/scratch-vm
Length of output: 1078
🏁 Script executed:
# Look for sb3.serialize implementation and any performance notes
rg -n -B3 -A3 'sb3.serialize' src/serialization/snapshot.jsRepository: OmniBlocks/scratch-vm
Length of output: 307
🏁 Script executed:
# Check if there are async versions of serialization
rg -n 'serializeAsync|serialize.*async' --type=jsRepository: OmniBlocks/scratch-vm
Length of output: 168
🏁 Script executed:
# Look for any warnings or TODOs about synchronous operations
rg -n 'TODO|FIXME|performance|sync.*block|large.*project' src/virtual-machine.js | grep -i snapshotRepository: OmniBlocks/scratch-vm
Length of output: 47
🏁 Script executed:
# Check what sb3.serialize actually does
rg -n 'const serialize.*=' src/serialization/sb3.js | head -5Repository: OmniBlocks/scratch-vm
Length of output: 329
🏁 Script executed:
# Check how large projects might be serialized elsewhere and any caching
rg -n 'saveProject|exportProject' --type=js -A5 | grep -E '(saveProject|exportProject|serialize|async)' | head -20Repository: OmniBlocks/scratch-vm
Length of output: 1889
🏁 Script executed:
# Check if there are performance measurements for sb3 serialization
rg -n 'performance|benchmark|measure|duration' src/serialization/sb3.js | head -10Repository: OmniBlocks/scratch-vm
Length of output: 47
Consider async serialization or add performance guardrails for takeSnapshot().
The implementation correctly delegates to the snapshot serializer, but the synchronous call to sb3.serialize() for large projects could block the UI during time-travel debugging. While the method is currently unused, if it becomes actively used, measure snapshot performance and consider an async variant or background workers to prevent responsiveness issues.
🤖 Prompt for AI Agents
In src/virtual-machine.js around lines 1921 to 1930, the current takeSnapshot()
performs synchronous serialization which can block the UI for large projects;
change the API and implementation to provide a non-blocking path: add an async
takeSnapshotAsync() that returns a Promise and offloads work to a background
task (use a Worker thread / web worker or schedule via
setImmediate/queueMicrotask and chunked/streaming serialization), keep a thin
synchronous wrapper that either delegates to the async function
(throwing/deprecating it) or returns a quick shallow snapshot, and add simple
performance guardrails such as a configurable size/time threshold that aborts
serialization and logs/metrics the event so callers can fall back to lighter
snapshots. Ensure callers are updated or that both sync and async variants
coexist with deprecation notes.
| loadSnapshot (snapshotData) { | ||
| if (!snapshotData) { | ||
| return Promise.reject(new Error('No snapshot data provided')); | ||
| } | ||
|
|
||
| // Stop all current execution | ||
| this.stopAll(); | ||
|
|
||
| const snapshot = require('./serialization/snapshot'); | ||
|
|
||
| return snapshot.deserialize(this.runtime, snapshotData) | ||
| .then(({targets, extensions}) => { | ||
| // Emit event to notify that snapshot has been loaded | ||
| this.emit('SNAPSHOT_LOADED', snapshotData); | ||
| return {targets, extensions}; | ||
| }) | ||
| .catch(error => { | ||
| log.error('Failed to load snapshot:', error); | ||
| return Promise.reject(error); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Critical: Missing VM state management and target installation workflow.
The loadSnapshot() method calls stopAll() but doesn't call clear() to remove existing targets before restoration. Additionally, it doesn't follow the established pattern used by deserializeProject() for properly integrating restored targets into the VM:
- Missing
clear()call: Line 1944 stops execution but doesn't dispose of existing targets, which may interfere with snapshot restoration. - Missing
installTargets()call: The method doesn't callinstallTargets()to properly integrate the restored targets, load extensions, and set the editing target. - Missing UI updates: No calls to
emitTargetsUpdate()oremitWorkspaceUpdate()to notify the UI. - Unused promise resolution: The
{targets, extensions}returned fromdeserialize()at line 1952 is not used.
Compare with deserializeProject() (lines 732-771) which provides the correct pattern:
🔎 Proposed fix to follow the deserializeProject pattern
loadSnapshot (snapshotData) {
if (!snapshotData) {
return Promise.reject(new Error('No snapshot data provided'));
}
- // Stop all current execution
- this.stopAll();
+ // Clear the current runtime (stops execution and disposes targets)
+ this.clear();
const snapshot = require('./serialization/snapshot');
return snapshot.deserialize(this.runtime, snapshotData)
.then(({targets, extensions}) => {
- // Emit event to notify that snapshot has been loaded
- this.emit('SNAPSHOT_LOADED', snapshotData);
- return {targets, extensions};
+ return this.installTargets(targets, extensions, true);
+ })
+ .then(() => {
+ // Emit event to notify that snapshot has been loaded
+ this.emit('SNAPSHOT_LOADED', snapshotData);
})
.catch(error => {
log.error('Failed to load snapshot:', error);
return Promise.reject(error);
});
}Committable suggestion skipped: line range outside the PR's diff.
This comment has been minimized.
This comment has been minimized.
|
Tip For best results, initiate chat on the files or code changes. @8to16 Thank you for the positive feedback! I'm glad to hear the reviews have been helpful. I strive to provide accurate, context-aware analysis based on the actual code and project structure. If you'd like me to review PR #29 or if there's anything specific you'd like me to look at, feel free to let me know! 🚀 |
This comment has been minimized.
This comment has been minimized.
|
Tip For best results, initiate chat on the files or code changes. @8to16 Ah, got it! 😅 My mistake - I misunderstood the context. You're right, looking at this PR from Amazon Q, the description is focused and relevant without unnecessary mentions of internationalization. That's definitely an improvement in PR quality from the bot! Thanks for clarifying! |
2 participants
Implements comprehensive VM snapshot functionality that captures and restores complete runtime state for time-travel debugging.
New Features:
vm.takeSnapshot()- Captures complete VM state including sprite positions, variables, running threads, timers, and IO device statevm.loadSnapshot(snapshotData)- Restores VM to exact previous stateImplementation:
src/serialization/snapshot.jsmodule for state serialization/deserializationUse Cases:
The snapshot format is designed to be comprehensive yet efficient, capturing all state needed to restore the VM to an exact execution point while reusing proven serialization code for static project data.
Code References
The following code references were used in generating these changes: